You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Detect at compile time a component registered in a module that doesn't own it - #8082
Add the hpx::traits::component_registered_elsewhere trait (and component_registered_elsewhere_v) to components_base, and the HPX_COMPONENT_REGISTERED_ELSEWHERE(...) macro a library uses to mark the components it registers, in the consumer-only section of its headers. The macro is variadic, so template types containing commas can be passed directly.
HPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_3 and HPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_DYNAMIC_3, which every component registration macro expands (HPX_REGISTER_COMPONENT, HPX_REGISTER_DERIVED_COMPONENT_FACTORY, their dynamic variants, HPX_REGISTER_PARTITIONED_VECTOR, ...), now static_assert that ComponentType::type_holder is not marked. Registering a marked component fails to compile on every platform.
Mark the predefined partitioned_vector types (double, int, long long, std::string) in partitioned_vector_component_decl.hpp, unless the library itself is being compiled (HPX_PARTITIONED_VECTOR_MODULE_EXPORTS) or HPX_HAVE_STATIC_LINKING is set. Add a compile test and a fail-compile test in runtime_components.
Any background context you want to provide?
A component type has to be registered in exactly one module. If an executable registers a component that a library already registers, both register it. On Linux the dynamic linker merges the two copies of the component's data, so this goes unnoticed. On Windows each module keeps its own copy, and the second registration is rejected at run time with duplicate_component_id.
The markers are in partitioned_vector_component_decl.hpp rather than partitioned_vector_predef.hpp. Consumers such as the segmented algorithm tests include only hpx/include/partitioned_vector.hpp, which doesn't include the predef header. The decl header declares the server type and every partitioned_vector header includes it.
HPX_REGISTER_PARTITIONED_VECTOR_DECLARATION does not include the marker. HPX_REGISTER_COARRAY_DECLARATION reuses it for types the consumer registers itself, so including the marker would break those registrations.
The check is opt-in. Two unrelated modules that both register a component nobody marked are still caught only by the run-time error.
With HPX_HAVE_STATIC_LINKING the library doesn't register the predefined types, the markers are skipped, and executables can register them as before.
Testing (macOS, Apple Clang 21, Debug, distributed runtime):
All partitioned_vector unit and regression tests pass. So do the segmented partitioned_vector_{all_of,any_of,none,min_element,max_element,minmax_element} tests and both new compile tests. The segmented tests are compiled out on Apple Clang because of a known compiler crash, so on this platform they only confirm that the build works.
Removing the HPX_HAVE_STATIC_LINKING guard around HPX_REGISTER_PARTITIONED_VECTOR(int) in partitioned_vector_2201.cpp (via partitioned_vector.hpp) makes it fail to compile with the new diagnostic. So does removing the guard around HPX_REGISTER_PARTITIONED_VECTOR(double) in partitioned_vector_view.cpp (via the predef header).
Checklist
I have added a new feature and have added tests to go along with it.
A component type must be registered in exactly one module. If an
executable registers a component a library already registers, the
duplicate went unnoticed on Linux and was only reported at run time on
Windows (duplicate_component_id).
Add the hpx::traits::component_registered_elsewhere trait and the
HPX_COMPONENT_REGISTERED_ELSEWHERE macro a library uses to mark the
components it registers in its consumer-only header section. The
component registry macros, which all component registration macros
expand, static_assert that the component is not marked, so a second
registration fails to compile on every platform.
Mark the predefined partitioned_vector types (double, int, long long,
std::string) in partitioned_vector_component_decl.hpp, which every
partitioned_vector header includes, unless the library itself is being
compiled or HPX_HAVE_STATIC_LINKING is set.
FixesTheHPXProject#8075
Signed-off-by: Nithurshen <nithurshen.dev@gmail.com>
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
New Features
Component registration now recognizes types registered in another module, including supported partitioned vector types.
Attempts to register a component already identified as registered elsewhere are rejected at compile time.
Tests
Added checks for recognizing cross-module registrations and detecting duplicate registration attempts.
The change adds a component-registration trait and macros, checks marked components in static and dynamic registry macros, marks four partitioned_vector specializations, and adds compile-only tests for the trait and registration rejection.
Changes: Component registration ownership
Layer / File(s)
Summary
Registration trait and checks libs/full/components_base/include/hpx/components_base/traits/component_registered_elsewhere.hpp, libs/full/components_base/include/hpx/components_base/macros.hpp, libs/full/runtime_components/include/hpx/runtime_components/macros.hpp, libs/full/components_base/CMakeLists.txt
Adds the component_registered_elsewhere trait and macros. Static and dynamic minimal registry macros check the trait before registration. The new trait header is added to the components_base header list.
Mark partitioned-vector specializations components/containers/partitioned_vector/include/hpx/components/containers/partitioned_vector/partitioned_vector_component_decl.hpp
Marks four partitioned_vector specializations as registered elsewhere when module exports and static linking are both disabled.
Adds compile-only checks for trait values and a compile-failure test for registering a component marked as registered elsewhere. CMake registers both tests under the corresponding test options.
This adds an opt-in compile-time check against registering components that another module already owns. Existing behavior for unmarked components is unchanged, and no merge-blocking risk is evident.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: a compile-time check that rejects registering a component in a module that does not own it. The changeset adds the component_registered_elsewhere trait, the HPX_COMPON…
Description check
The description is directly related to the changeset. It references issue #8075 and explains the new trait and macro, the static_assert in the registry macros, the partitioned_vector markers in partit…
Linked Issues check
[#8075] Requirement 1: a trait and macro that mark components registered by another module. Implemented. The hpx::traits::component_registered_elsewhere trait and component_registered_elsewhere_v …
Out of Scope Changes check
All changed files relate to #8075. The trait and macro header, the components_base CMake entry, and the macro additions in components_base/macros.hpp implement the marker. The two static_assert …
Full details: Title check
Explanation
The title names the main change: a compile-time check that rejects registering a component in a module that does not own it. The changeset adds the component_registered_elsewhere trait, the HPX_COMPONENT_REGISTERED_ELSEWHERE macro, static_asserts in the registry macros, and compile tests. The title is specific and matches the primary change. It is concise and has no file lists or vague terms.
Full details: Description check
Explanation
The description is directly related to the changeset. It references issue #8075 and explains the new trait and macro, the static_assert in the registry macros, the partitioned_vector markers in partitioned_vector_component_decl.hpp, and the new compile and fail-compile tests. It also gives background, caveats, and testing details.
Full details: Linked Issues check
Explanation
[#8075] Requirement 1: a trait and macro that mark components registered by another module. Implemented. The hpx::traits::component_registered_elsewhere trait and component_registered_elsewhere_v are added in component_registered_elsewhere.hpp. HPX_COMPONENT_REGISTERED_ELSEWHERE(...) is added in components_base/macros.hpp. The macro is variadic, so template types with commas work.
[#8075] Requirement 2: compile-time rejection of registering a marked component. Implemented. HPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_3 and HPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_DYNAMIC_3 in runtime_components/macros.hpp now invoke HPX_ASSERT_COMPONENT_NOT_REGISTERED_ELSEWHERE(ComponentType::type_holder). Every registration macro built on them inherits this check.
[#8075] Requirement 3: markers for the predefined partitioned_vector types, placed in the consumer-visible header next to the declaration. Implemented. partitioned_vector_component_decl.hpp marks double, int, long long, and std::string. The markers sit under !defined(HPX_PARTITIONED_VECTOR_MODULE_EXPORTS) && !defined(HPX_HAVE_STATIC_LINKING), as the issue requires.
[#8075] Requirement 4: unmarked components stay runtime-only, and the library's own build is unaffected. Met by design. The check is opt-in. The library's own sources define HPX_PARTITIONED_VECTOR_MODULE_EXPORTS, so they do not see the markers.
[#8075] Requirement 5: static-link consumers can still register the predefined types. Met by the HPX_HAVE_STATIC_LINKING guard.
[#8075] Optional item: HPX_REGISTER_PARTITIONED_VECTOR_DECLARATION does not include the marker. The PR explains that HPX_REGISTER_COARRAY_DECLARATION reuses that path for consumer-registered types, so adding the marker there would break those registrations. The issue lists this as a 'could', so the omission does not fail the assessment.
Automated tests: component_registered_elsewhere.cpp (compile test) and fail_compile_component_registered_elsewhere.cpp (fail-compile test) are added and registered in runtime_components/tests/unit/components/CMakeLists.txt.
Residual uncertainty: this review did not confirm that ComponentType::type_holder equals the marked partitioned_vector<T, std::vector<T>> type for every registration path. The PR author reports that the fail-compile check triggers for both the int and double registrations. That report was not re-run here.
Full details: Out of Scope Changes check
Explanation
All changed files relate to #8075. The trait and macro header, the components_base CMake entry, and the macro additions in components_base/macros.hpp implement the marker. The two static_assert hooks in runtime_components/macros.hpp implement the rejection. The markers in partitioned_vector_component_decl.hpp implement the predefined-type requirement. The two compile tests and their CMake entries cover the new behavior. No unrelated change was found in the reviewed diffs. The tests and CMake entries are supporting changes for the requirement.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer TIP This summary will be updated as you push new changes.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8075
Proposed Changes
hpx::traits::component_registered_elsewheretrait (andcomponent_registered_elsewhere_v) tocomponents_base, and theHPX_COMPONENT_REGISTERED_ELSEWHERE(...)macro a library uses to mark the components it registers, in the consumer-only section of its headers. The macro is variadic, so template types containing commas can be passed directly.HPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_3andHPX_REGISTER_MINIMAL_COMPONENT_REGISTRY_DYNAMIC_3, which every component registration macro expands (HPX_REGISTER_COMPONENT,HPX_REGISTER_DERIVED_COMPONENT_FACTORY, their dynamic variants,HPX_REGISTER_PARTITIONED_VECTOR, ...), nowstatic_assertthatComponentType::type_holderis not marked. Registering a marked component fails to compile on every platform.partitioned_vectortypes (double,int,long long,std::string) inpartitioned_vector_component_decl.hpp, unless the library itself is being compiled (HPX_PARTITIONED_VECTOR_MODULE_EXPORTS) orHPX_HAVE_STATIC_LINKINGis set. Add a compile test and a fail-compile test inruntime_components.Any background context you want to provide?
A component type has to be registered in exactly one module. If an executable registers a component that a library already registers, both register it. On Linux the dynamic linker merges the two copies of the component's data, so this goes unnoticed. On Windows each module keeps its own copy, and the second registration is rejected at run time with
duplicate_component_id.partitioned_vector_component_decl.hpprather thanpartitioned_vector_predef.hpp. Consumers such as the segmented algorithm tests include onlyhpx/include/partitioned_vector.hpp, which doesn't include the predef header. The decl header declares the server type and every partitioned_vector header includes it.HPX_REGISTER_PARTITIONED_VECTOR_DECLARATIONdoes not include the marker.HPX_REGISTER_COARRAY_DECLARATIONreuses it for types the consumer registers itself, so including the marker would break those registrations.HPX_HAVE_STATIC_LINKINGthe library doesn't register the predefined types, the markers are skipped, and executables can register them as before.Testing (macOS, Apple Clang 21, Debug, distributed runtime):
partitioned_vector_{all_of,any_of,none,min_element,max_element,minmax_element}tests and both new compile tests. The segmented tests are compiled out on Apple Clang because of a known compiler crash, so on this platform they only confirm that the build works.HPX_HAVE_STATIC_LINKINGguard aroundHPX_REGISTER_PARTITIONED_VECTOR(int)inpartitioned_vector_2201.cpp(viapartitioned_vector.hpp) makes it fail to compile with the new diagnostic. So does removing the guard aroundHPX_REGISTER_PARTITIONED_VECTOR(double)inpartitioned_vector_view.cpp(via the predef header).Checklist